Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Deleting it sounds great (totally not me talking about removing it being the reason for you doing it) but I'd like to get some visibility for this anyways, cc @rust-lang/lang @rust-lang/compiler

Updated rustc_ast|hir_pretty to use #[rustc_box]. I restored ExprKind::Box for now since Clippy and R-A use it but it isn't constructed any more

oops, misclick
If the changes to clippy and ra are very small you can just make them here - if they aren't you can PR it on their repos and block this PR on that. I think it's not worth it to have two PRs here.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

The eventual removal of box syntax has been my long term goal (#87781, #88316, #97293, #97494, #99066, #99577, #100194, etc all worked in that direction), but I've been avoiding doing the last step. It's great to see this being proposed.

I think the presence of box syntax causes confusion among people in the sense that they think it is going to be stabilized, which it won't.

The last triage comment from the lang team has said that the lang team waits for stabilization-track features (from my reading those are features that are more likely going to be stabilized than box will) that overlaps with box syntax before box syntax is removed.

I think given boxing is sort-of available on stable, this is mostly resolved. Yes, there is no guarantee for vec to be optimized in such a fashion, but on the other hand, removing the optimization will have performance impacts on user code that are too large. It might be a good idea to talk to potential users of placement new if their needs are met by this (code link).

Also note that the functionality itself is not deleted, just the box syntax for constructing boxes. #[rustc_box] is still around. I have waited with the box_syntax removal step because I wanted my MCP on builtin# to be seconded and implemented before proposing this, but I don't think it's a hard blocker for removal, it would just have been nicer to point users towards builtin#box(foo) instead as it wouldn't have needed two feature gates.

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/rust-analyzer

cc @rust-lang/wg-rls-2

Not sure about the mir-opt test failures, I blessed them locally but CI has different results. I might try #[rustc_box]ing them here to restore the old behaviour

Some comments on the PR itself:

• As implemented right now, this change makes the language accepted by the AST parsing less tolerant. In other words, it could cause breakage for macro users where the attribute macro does something with the box to make it disappear, or it could cause issues for people who have put box-syntax using stuff behind #[cfg(false)] or similar. I don't think this is happening, and I doubt going the forward compat lint route is needed, but it has caused breakages in the past, see Pre-expansion gate some more things #64672 and Partially revert the early feature-gatings added in #65742. #66004. I wonder if a crater run would be a good idea or not. In the worst case, one could move the error to AST->HIR lowering and emit a forward compat lint, but I doubt it's needed. Edit: PR Reenable disabled early syntax gates as future-incompatibility lints #99935 has added a forward compat lint for the #[cfg(FALSE)] fn foo() { box 0; } case, so I'd say this issue is resolved.

• box syntax might be quite used outside of rustc. It might make sense, at least for a few release cycles, to emit an actual box-syntax specific error with a rustfix-able suggestion to use Box::new. That would give users an automated way to migrate away. The usage of box syntax in the wild is non zero, and it's not trivial to build a regex that converts box FooExpr(FooBar{ hi: hello(), }) to the Box::new equivalent.

@leonardo-m you can still use #[rustc_box] Box::new(...) if you really want, which is equivalent to using box syntax... but ideally you'd use one of the alternatives, like the one you wrote about that uses vec![]. It's odd that that is really slower than box syntax. Yes, it casts the pointer around a bit but that should be very easily optimized out.

Box::new is fine for 99% of cases, without causing performance impacts or stack overflows. For the remaining 1%, issuing a good suggestion for an alternative is hard. But at least, affected codebases can run cargo +nightly fix --broken-code so they have an easy way to migrate away from box syntax (I tried it out today on an old checkout of checkr, it works really great).

@saethlin I don't know your level of experience about this stuff. An array often offers more compile-time information to the back-end, this in several situations allows LLVM to optimize away bound tests and this allows LLVM to vectorize some code, increasing code performance in those kernels. I don't think you can open an issue that tracks this because this can't be fixed unless LLVM itself becomes "sufficiencly smart" about reasoning about lengths. In theory the information LLVM has is about the same, in practice the generated asm differs, once you have two or three nested loops.

@est31 thank you, sorry for bothering you people, you're doing good work... I will try the alternatives.

@leonardo-m I have a fair bit of experience with this stuff. I and the comments elsewhere in this PR are suggesting you use vec! to construct a boxed slice, not that you use a Vec<T> instead of Box<[T; N]>. I thought you were saying this suggestion causes an unacceptable performance regression. If it does, we should have an issue to track that.

I always wondered why Box::new isn't based on Box::new_in with just the global allocator given (yes, I know that GlobalAlloc is not Allocator). Do you know where I can find enlightening discussion on this?